Fix/kanban url#77
Conversation
This commit introduces the main team dashboard page, including: - Integrated Kanban board for task management with drag-and-drop functionality. - Member section to view and manage team members. - API integrations and TanStack Query hooks for group, task, and task list operations. - Responsive design for various screen sizes (desktop, tablet, mobile). - Updated .gitignore to exclude dev-specific session injection route.
Summary of ChangesHello @Jieunsse, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily addresses a large-scale file and folder restructuring, seemingly to recover or re-upload source code that was previously missing after a folder move. Beyond this structural change, it significantly enhances the Kanban board's user experience by preventing unintended drag-and-drop actions on interactive elements and refining the task management logic with optimistic updates for checkbox changes. Additionally, it improves sidebar navigation by enabling direct access to the user's profile page. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request primarily focuses on renaming files and making minor adjustments to Kanban board functionality and sidebar navigation. The file renames are extensive, suggesting a restructuring of the project. There are also changes to how Kanban tasks are handled, specifically regarding optimistic updates for item checked states and simplified status changes. Additionally, a new AddTeamSidebarWrapper component was introduced and integrated into the addteam layout, and the main SidebarWrapper now includes navigation to the user's mypage. The changes are generally well-implemented, but some areas could benefit from minor improvements in code clarity and consistency.
| // 드래그로 컬럼 이동 시 컬럼 위치만 localStorage에 저장 (체크박스 상태는 변경하지 않음) | ||
| const handleStatusChange = useCallback( | ||
| async (taskId: string, fromStatus: KanbanStatus, toStatus: KanbanStatus) => { | ||
| (taskId: string, fromStatus: KanbanStatus, toStatus: KanbanStatus) => { | ||
| if (fromStatus === toStatus) return; | ||
|
|
||
| const task = tasks.find((t) => t.id === taskId); | ||
| const taskListId = Number(taskId); | ||
|
|
||
| // 항목 유무와 관계없이 컬럼 위치를 localStorage에 저장 | ||
| setStoredStatus(groupId, taskListId, toStatus); | ||
|
|
||
| // 진행중으로 이동하거나 항목이 없으면 API 호출 없이 종료 (위치는 이미 저장됨) | ||
| if (!task || task.items.length === 0 || toStatus === 'inProgress') return; | ||
|
|
||
| try { | ||
| if (toStatus === 'done') { | ||
| // 모든 항목 완료 처리 | ||
| await Promise.all( | ||
| task.items.map((item) => | ||
| updateTask(groupId, taskListId, Number(item.id), { done: true }), | ||
| ), | ||
| ); | ||
| } else if (toStatus === 'todo') { | ||
| // 모든 항목 미완료 처리 | ||
| await Promise.all( | ||
| task.items.map((item) => | ||
| updateTask(groupId, taskListId, Number(item.id), { done: false }), | ||
| ), | ||
| ); | ||
| } | ||
| } finally { | ||
| // 성공/실패 관계없이 쿼리를 무효화하여 실제 서버 상태로 동기화 | ||
| await queryClient.invalidateQueries({ | ||
| queryKey: taskListKeys.detail(groupId, taskListId, today), | ||
| }); | ||
| } | ||
| setStoredStatus(groupId, Number(taskId), toStatus); | ||
| }, |
There was a problem hiding this comment.
The previous implementation of handleStatusChange included logic to update task items' done status via API calls when moving tasks between 'done' and 'todo' columns. The current change removes this logic, making handleStatusChange solely responsible for updating the local storage status. This means that moving a task to 'done' or 'todo' will no longer automatically update the done status of its items on the backend. This could lead to a discrepancy between the UI's representation of task completion and the actual backend state, potentially causing data inconsistencies or unexpected behavior for users. If the intention is to only update the local storage, this should be clearly documented or the API calls should be re-introduced if the backend state needs to reflect the column change.
const handleStatusChange = useCallback(
async (taskId: string, fromStatus: KanbanStatus, toStatus: KanbanStatus) => {
if (fromStatus === toStatus) return;
setStoredStatus(groupId, Number(taskId), toStatus);
// Re-introduce API calls if backend state needs to reflect column changes
const task = tasks.find((t) => t.id === taskId);
const taskListId = Number(taskId);
if (!task || task.items.length === 0 || toStatus === 'inProgress') return;
try {
if (toStatus === 'done') {
await Promise.all(
task.items.map((item) =>
updateTask(groupId, taskListId, Number(item.id), { done: true }),
),
);
} else if (toStatus === 'todo') {
await Promise.all(
task.items.map((item) =>
updateTask(groupId, taskListId, Number(item.id), { done: false }),
),
);
}
} finally {
await queryClient.invalidateQueries({
queryKey: taskListKeys.detail(groupId, taskListId, today),
});
}
},
[groupId, tasks, today, queryClient]
);| [groupId, today, queryClient], | ||
| ); |
There was a problem hiding this comment.
The dependency array for handleItemCheckedChange includes groupId, today, and queryClient. While groupId and queryClient are stable, today is derived from new Date().toISOString().split('T')[0] and will change on every render, causing handleItemCheckedChange to be recreated unnecessarily. This can lead to performance issues or unexpected behavior if handleItemCheckedChange is used as a dependency in other hooks. Consider memoizing today or passing it as an argument if it's meant to be dynamic.
| [groupId, today, queryClient], | |
| ); | |
| [groupId, queryClient] | |
| ); |
| font-size: 12px; | ||
| font-weight: 400; |
There was a problem hiding this comment.
It's generally a good practice to define font sizes and weights in a more centralized theme or utility file rather than directly in component-specific CSS modules. This helps maintain consistency across the application and makes it easier to manage design tokens.
| @@ -1,4 +1,5 @@ | |||
| import { useState } from 'react'; | |||
| import type React from 'react'; | |||
There was a problem hiding this comment.
While type React from 'react' is technically correct, the more common and idiomatic way to import types from React is import type { PointerEvent } from 'react'; or import type { ReactNode } from 'react'; if you need specific types. Importing the entire React namespace as a type is less common.
import { useState } from 'react';
import type { PointerEvent } from 'react';| 'use client'; | ||
|
|
||
| import { useRouter } from 'next/navigation'; | ||
| import { Sidebar } from '@/components/sidebar'; | ||
|
|
||
| export default function AddTeamSidebarWrapper() { | ||
| const router = useRouter(); | ||
|
|
||
| return <Sidebar onProfileClick={() => router.push('/mypage')} />; | ||
| } |
There was a problem hiding this comment.
The AddTeamSidebarWrapper component is a very thin wrapper around Sidebar that only passes onProfileClick. If this is the only difference, consider if this wrapper is truly necessary. It might be more straightforward to pass the onProfileClick prop directly where Sidebar is used in AddTeamLayout, or to abstract the onProfileClick logic into a custom hook if it's reused across multiple sidebar instances with different behaviors.
1 participant
Summary
Issue
Scope
포함
특이사항